-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[on hold] Verify, that grpc can be used for Openshift probes #1212
Conversation
...entelemetry-reactive/src/test/java/io/quarkus/ts/opentelemetry/reactive/OpenShiftGrpcIT.java
Outdated
Show resolved
Hide resolved
...entelemetry-reactive/src/test/java/io/quarkus/ts/opentelemetry/reactive/OpenShiftGrpcIT.java
Outdated
Show resolved
Hide resolved
...entelemetry-reactive/src/test/java/io/quarkus/ts/opentelemetry/reactive/OpenShiftGrpcIT.java
Outdated
Show resolved
Hide resolved
.withProperty("quarkus.opentelemetry.tracer.exporter.otlp.endpoint", jaeger::getCollectorUrl); | ||
.withProperty("quarkus.opentelemetry.tracer.exporter.otlp.endpoint", jaeger::getCollectorUrl) | ||
.withProperty("quarkus.openshift.readiness-probe.grpc-action", "9000:io.quarkus.example.PongService") | ||
.withProperty("quarkus.openshift.liveness-probe.grpc-action", "9000:io.quarkus.example.PongService"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to specify also method? e.g. does this call sayPong
or returnLastTraceId
? and can we set different method for liveness and readiness probe so that we have verified it works when liveness != readiness?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a good question.
According to the specification[1], a cluster should use Check method of the service automatically [2] and "unimplemented health checking protocol <...> considered a probe failure". I would say our cluster doesn't use grpc probes, but they are enabled by default since 1.24[3] and we use 1.25 and there were no complaints during deployment.
[1] https://github.com/grpc/grpc/blob/master/doc/health-checking.md
[2] https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/#define-a-grpc-liveness-probe
[2] https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/#feature-gates-for-graduated-or-deprecated-features
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if you added SmallRye Health this gRPC Health service would be generated automatically for you right? Do we have a way to verify that gRPC probes were actually used? e.g. right now we check that template is there and we assume that pods are ready because they were not killed and we are running tests, but we don't check that template doesn't contain also HTTP probes that could be used?
Unlike you, I didn't test it so you can lead me there, but I'd really like to:
- check that it fails when such Health service is not available (e.g. custom health check that passes for liveness but not for readiness and we check that liveness succeeded but readiness failed)
- check it doesn't fail when it is available and pod is ready and there are no other probes that could be used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I verified, that the deployment fails, if the service is misspelled (eg HelathService
). At the same time, I was not able to force deployment to fail because of error in liveness probe (see the code for an example). I will create a stand-alone reproducer and an issue on Tuesday
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue: quarkusio/quarkus#33219
...entelemetry-reactive/src/test/java/io/quarkus/ts/opentelemetry/reactive/OpenShiftGrpcIT.java
Show resolved
Hide resolved
34933bd
to
870f1a7
Compare
870f1a7
to
2aea79a
Compare
run tests |
I thought we need FW release. |
Since adding custom gRPC Health endpoints is not supported[1] I am closing this PR |
Summary
See quarkusio/quarkus#32113 for details.
Requires quarkus-qe/quarkus-test-framework#772
Waits for quarkusio/quarkus#33219 to be fixed, since right now it is not possible for user to create a custom health service using GRPC.
Please select the relevant options.
run tests
phrase in comment)Checklist: